Skip to content

Conversation

@adamziel
Copy link
Collaborator

@adamziel adamziel commented Nov 28, 2024

What is this PR doing?

Adds PHP 8.4 to Playground.

Also updates the dependencies:

  • Upgrades libz to 1.2.13 as PHP 8.4 was no longer compatible with 1.2.5 we've used before.
  • Adds libopenssl 1.1.1 required by PHP 8.4. We still keep 1.1.0h in the repo as I had issues building PHP 7.0 with 1.1.1.

Testing Instructions

  1. Go to http://localhost:5400/website-server/ and switch the PHP version to 8.4.
  2. Confirm that WordPress is functional and you can update the site in the site editor
  3. Go to /phpinfo.php, confirm the php version it 8.4
  4. Update settings to run with additional PHP extensions and networking support
  5. Confirm that WordPress is functional, you can update the site in the site editor, and the plugin directory shows plugins and you can install them
  6. Go to /phpinfo.php, confirm the php version it 8.4
  7. In CLI, run PHP=8.4 nx run php-wasm-cli:start -- -i and confirm the PHP version presented on the screen is 8.4

@adamziel
Copy link
Collaborator Author

adamziel commented Dec 4, 2024

Oh no, new PHP release = a bunch of new functions to add to the asyncify list 😢

@brandonpayton
Copy link
Member

brandonpayton commented Dec 17, 2024

I ran the tests with longer stack traces, and a big thing I see missing in the asyncify declarations is php_execute_script_ex which was added within the last year:
php/php-src@c149b4f#diff-2978fe1c2c45b4eca89dc476376ddc7193bc4e5e7fff0c7d1c465f057b35a5e6R2434

🤞 hopefully we won't need to add too many more.

I'm fixing my docker setup and planning to rebuild PHP 8.4 with this function added.

@brandonpayton
Copy link
Member

I had trouble getting the builds to run with errors like:

#8 1.435 Err:5 http://ports.ubuntu.com/ubuntu-ports lunar Release             
#8 1.435   404  Not Found [IP: 185.125.190.39 80]                        
#8 1.663 Err:6 http://ports.ubuntu.com/ubuntu-ports lunar-updates Release
#8 1.663   404  Not Found [IP: 185.125.190.39 80]  
#8 1.885 Err:7 http://ports.ubuntu.com/ubuntu-ports lunar-backports Release
#8 1.885   404  Not Found [IP: 185.125.190.39 80]                            
#8 2.113 Err:8 http://ports.ubuntu.com/ubuntu-ports lunar-security Release    
#8 2.113   404  Not Found [IP: 185.125.190.39 80]             
#8 2.116 Reading package lists...
#8 2.120 E: The repository 'http://ports.ubuntu.com/ubuntu-ports lunar Release' does not have a Release file.                                                
#8 2.120 E: The repository 'http://ports.ubuntu.com/ubuntu-ports lunar-updates Release' does not have a Release file.                                        
#8 2.120 E: The repository 'http://ports.ubuntu.com/ubuntu-ports lunar-backports Release' does not have a Release file.                                      
#8 2.120 E: The repository 'http://ports.ubuntu.com/ubuntu-ports lunar-security Release' does not have a Release file.

and

#10 [ 6/12] RUN git clone https://github.com/emscripten-core/emsdk.git &&
./emsdk/emsdk install 3.1.61 &&     
/root/emsdk/emsdk activate 3.1.61          #10 0.110 Cloning into 'emsdk'...                                                                                                                            
#10 11.34 error: RPC failed; HTTP 503 curl 22 The requested URL returned error: 503                                                                          
#10 11.34 fatal: expected flush after ref listing
#10 ERROR:  process "/bin/bash -c git clone https://github.com/emscripten-core/emsdk.git &&
./emsdk/emsdk install 3.1.61 &&     
/root/emsdk/emsdk activate 3.1.61" did not complete successfully: exit code: 128  

To fix them, I had to:

  • update the ubuntu distribution to the newer "noble" release
  • update the emsdk version to 3.1.74

I'll create another PR for those changes. It seems fine to upgrade the versions we use for build.

Now there is a remaining build error but much later in the process. This line fails:
/root/replace.sh $'s/if\s*\(\s*["\']string["\']\s*===\s*typeof Module\[["\']websocket["\']\]\[["\']url["\']\]\s*\)/if("function"===typeof Module["websocket"]["url"]) {\nurl = Module["websocket"]["url"](...arguments);\n}else if ("string" === typeof Module["websocket"]["url"])/g' \ /root/output/php.js; \

The code must have changed. Planning to look at that.

@brandonpayton
Copy link
Member

Now there is a remaining build error but much later in the process. This line fails:
/root/replace.sh $'s/if\s*(\s*["']string["']\s*===\stypeof Module[["']websocket["']][["']url["']]\s)/if("function"===typeof Module["websocket"]["url"]) {\nurl = Module["websocket"]"url";\n}else if ("string" === typeof Module["websocket"]["url"])/g' \ /root/output/php.js; \

The code must have changed. Planning to look at that.

I have a local patch that seems good for this, but now there is an issue due to a change in the Emscripten-generated JS. With Emscripten 3.1.74, our ExitStatus patch conflicts because the output changed from declaring ExitStatus as a function to declaring it as a class. Functions declared anywhere in a scope are available to all statements within a scope, even if the statements precede the function declaration. The same is not true for classes, so I started running into errors that were something like "ExitStatus cannot be referenced before initialized".

I think we just need to pinpoint the location of the class declaration in the JS file and insert our overriding definition immediately after that.

This is quite a lot of yak-shaving by now, but I'm planning to continue with these upgrades under a separate PR because they'll need to happen sometime and I'm unable to build php-wasm without them.

@brandonpayton
Copy link
Member

@adamziel, I pushed what I think may be a sufficient asyncify addition for PHP 8.4 to work but am personally unable to rebuild PHP 8.4 until #2116 lands due to the missing version errors mentioned above.

If php-wasm builds are still working for you, please feel free to rebuild and test PHP 8.4 asyncify again in the meantime. Otherwise, I am happy to rebuild after #2116 lands.

@adamziel
Copy link
Collaborator Author

adamziel commented Jan 9, 2025

@brandonpayton I've rebuilt but it keeps failing. I've added a few more functions and am still seeing those failures. We might need better debugging facilities to make progress here. Alternatively, there's a brute-force approach where we'd list all C functions added in 8.4 and, if it works, bisect to find the right one:

php/php-src@PHP-8.3.16...PHP-8.4.0

@adamziel
Copy link
Collaborator Author

adamziel commented Jan 9, 2025

I'm wrong. I rebuilt the web version and tested the node version. Your fix works great @brandonpayton. I'll push the rebuilt files soon

@adamziel adamziel merged commit a0c7b02 into trunk Jan 9, 2025
9 of 10 checks passed
@adamziel adamziel deleted the php-8.4 branch January 9, 2025 16:20
@brandonpayton
Copy link
Member

Thanks for handling this and all the time rebuilding and testing, @adamziel!

brandonpayton added a commit that referenced this pull request Mar 7, 2025
Note: This work is continued from a PR started under the WordPress GH
org.

There are a couple of reasons for this upgrade:

1. I am having trouble rebuilding php-wasm on my current system.
References to older ubuntu images and emscripten versions [has given me
trouble](#2038 (comment)).
2. We want [this Emscripten
fix](emscripten-core/emscripten#22932) from
@jeroenpf and @bgrgicak which was first included in Emscripten 3.1.73.

This PR:
- Bumps the ubuntu docker image version due to build errors related to
missing image. Referencing a newer ubuntu image seemed to fix the
"missing image" errors.
- Bumps Emscripten version
- Adjusts Playground-specific patches to Emscripten output since the
latest breaks that patching.

- CI (once we can run GH actions on A8C's GHE instance)

---------

Co-authored-by: Brandon Payton <[email protected]>
bgrgicak added a commit that referenced this pull request May 7, 2025
## Motivation for the change, related issues

[Emscripten merged a fix for
`statfs`](emscripten-core/emscripten#23381) in
`4.0.1`, so this PR updates Emscripten to allow us to run `statfs` in
PHP functions like `disk_total_space`.

## Implementation details

This PR updates the version of Emscripten to `4.0.5`, rebuilds all
PHP-wasm libraries, and recompiles PHP-wasm.

While rebuilding PHP-wasm libraries there was a bug in the `libcurl`
build script because it expected `libopenssl` to be located in
`libopenssl/asyncify/dist`, but in reality `libopenssl` was located in
`libopenssl/asyncify/dist/1.1.0h` (similarly for JSPI).
We have two versions of `libopenssl` because it [was requires for PHP
7.0](#2038).
This PR drops `libopenssl 1.1.0h` support because [Playground doesn't
support PHP 7.0
anymore.](Automattic/wordpress-playground-private#74)

## Testing Instructions (or ideally a Blueprint)

- CI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants